Skip to content

Update to use new test data models#713

Open
skottmckay wants to merge 9 commits into
mainfrom
skottmckay/UpdateToNewTestDataModels
Open

Update to use new test data models#713
skottmckay wants to merge 9 commits into
mainfrom
skottmckay/UpdateToNewTestDataModels

Conversation

@skottmckay
Copy link
Copy Markdown
Collaborator

@skottmckay skottmckay commented May 16, 2026

Also Fix test issues.
Some due to new models.
Some due to not running tests.

The Utils init of whether test data is available doesn't seem to fire before we check the value in the 'skip if not available' attribute. Changed to use static ctor to ensure it's inited.

@skottmckay
Update to use new test data models
ecb060c 
Fix test issues. Some due to new modle. Some due to not running tests.
@vercel
Copy link
Copy Markdown

vercel Bot commented May 16, 2026 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
foundry-local Ready Ready Preview, Comment May 24, 2026 7:46am

Request Review

Copilot AI reviewed May 16, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates test suites and docs to align with the new shared test-data model naming and adjusts C# integration test setup/streaming handling.

Changes:

  • Renames embedding test model aliases across Rust, Python, JS, and C# tests.
  • Updates C# embedding expected model/value assertions and initializes test utilities via a static constructor.
  • Adjusts C# streaming chat/tool tests to stop before unsupported usage responses.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
sdk/rust/tests/integration/common/mod.rs Updates shared Rust embedding test model alias.
sdk/python/test/README.md Updates documented Whisper test variant version.
sdk/python/test/conftest.py Updates shared Python embedding test model alias.
sdk/js/test/testUtils.ts Updates shared JS embedding test model alias.
sdk/js/test/openai/embeddingClient.test.ts Updates embedding cache assertion text.
sdk/cs/test/FoundryLocal.Tests/Utils.cs Moves test utility initialization into a static constructor and trims whitespace.
sdk/cs/test/FoundryLocal.Tests/EmbeddingClientTests.cs Updates C# embedding variant references and known expected value.
sdk/cs/test/FoundryLocal.Tests/ChatCompletionsTests.cs Breaks streaming test loops when stop finish reason is reached.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread sdk/python/test/README.md
baijumeswani
baijumeswani previously approved these changes May 16, 2026
View reviewed changes
@skottmckay
Copy link
Copy Markdown
Collaborator Author

skottmckay commented May 18, 2026

@copilot why are tests failing to find the new model? are we not checking out the latest test-data-shared?

  1. Embedding Client Tests
    should generate embedding:
    AssertionError: qwen3-embedding-0.6b-generic-cpu should be cached: expected undefined not to be undefined
    at Context. (test\openai\embeddingClient.test.ts:16:91)

Copy link
Copy Markdown
Contributor

Copilot AI commented May 18, 2026

@copilot why are tests failing to find the new model? are we not checking out the latest test-data-shared?

  1. Embedding Client Tests
    ...

The test-data-shared checkout is fine. The issue was a typo in sdk/js/test/testUtils.ts — EMBEDDING_MODEL_ALIAS was set to 'qwen3-embedding-0.6b' instead of 'qwen3-embedding-0.6b-generic-cpu', so the lookup cachedModels.find(m => m.alias === EMBEDDING_MODEL_ALIAS) always returned undefined. Python, Rust, and C# all already had the full -generic-cpu suffix. Fixed in commit 0a3859c.

@skottmckay
Copy link
Copy Markdown
Collaborator Author

skottmckay commented May 18, 2026

@copilot or are we missing the version number from the model id?

these were the changes

Copy link
Copy Markdown
Contributor

Copilot AI commented May 18, 2026

@copilot or are we missing the version number from the model id?

these were the changes

No version number is needed. The alias field is always the variant alias without version (e.g., qwen3-embedding-0.6b-generic-cpu); the version is only in the id field (e.g., qwen3-embedding-0.6b-generic-cpu:1). The test searches by m.alias, so :1 doesn't factor in.

The actual cause: commit d00a002 changed EMBEDDING_MODEL_ALIAS from 'qwen3-embedding-0.6b-generic-cpu' (the specific variant alias in test-data-shared) to 'qwen3-embedding-0.6b' (the short parent alias used by sample apps). That short alias doesn't match any variant's alias field in the catalog, so cachedModels.find(m => m.alias === EMBEDDING_MODEL_ALIAS) returns undefined. Commit 0a3859c restores it to 'qwen3-embedding-0.6b-generic-cpu'.

@skottmckay
Copy link
Copy Markdown
Collaborator Author

skottmckay commented May 18, 2026

@copilot C# tests passed. python, js and rust tests failed

@skottmckay
Fix EMBEDDING_MODEL_ALIAS to use correct catalog alias qwen3-embeddin…
5619b5a 
…g-0.6b

The model variant ID is qwen3-embedding-0.6b-generic-cpu:1 but the catalog
alias is qwen3-embedding-0.6b (without -generic-cpu). C# tests passed because
they lookup by variant ID directly. Python/JS/Rust tests failed because they
search by alias == 'qwen3-embedding-0.6b-generic-cpu' which doesn't match.

- JS testUtils.ts: qwen3-embedding-0.6b-generic-cpu -> qwen3-embedding-0.6b
- Python conftest.py: qwen3-embedding-0.6b-generic-cpu -> qwen3-embedding-0.6b
- Rust common/mod.rs: qwen3-embedding-0.6b-generic-cpu -> qwen3-embedding-0.6b
- JS embeddingClient.test.ts: use EMBEDDING_MODEL_ALIAS in error message
- README: fix alias column in test models table

Agent-Logs-Url: https://github.com/microsoft/Foundry-Local/sessions/785d0b4d-9f16-49a5-b984-bdfaa025e275

Co-authored-by: skottmckay <979079+skottmckay@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

Copilot AI commented May 18, 2026

@copilot C# tests passed. python, js and rust tests failed

The root cause: ecb060c set EMBEDDING_MODEL_ALIAS to qwen3-embedding-0.6b-generic-cpu for Python/Rust, and my commit 0a3859c reverted JS back to the same wrong value. The actual catalog alias is qwen3-embedding-0.6b — the -generic-cpu suffix is part of the variant name/ID (qwen3-embedding-0.6b-generic-cpu:1), not the alias. C# tests passed because they use GetModelVariantAsync("qwen3-embedding-0.6b-generic-cpu:1") (lookup by ID), while Python/JS/Rust search by m.alias which never matched.

Fixed in commit 5619b5a: changed EMBEDDING_MODEL_ALIAS to qwen3-embedding-0.6b in all three SDKs, updated the hardcoded error message in the JS test, and corrected the alias column in the README.

@skottmckay
Copy link
Copy Markdown
Collaborator Author

skottmckay commented May 18, 2026

@copilot alias is qwen3-embedding-0.6b as the model is coming from the catalog. if it wasn't the alias is closer to the model id

@skottmckay skottmckay enabled auto-merge (squash) May 18, 2026 04:00
@skottmckay skottmckay requested a review from baijumeswani May 22, 2026 23:27
baijumeswani
baijumeswani previously approved these changes May 22, 2026
View reviewed changes
@skottmckay skottmckay requested a review from baijumeswani May 24, 2026 07:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

Copilot code review Copilot Copilot left review comments

@baijumeswani baijumeswani Awaiting requested review from baijumeswani

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@skottmckay @baijumeswani